-
Notifications
You must be signed in to change notification settings - Fork 239
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
RFC: npm audit licenses #182
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about individual package overrides?
while licensee supports using community overrides (which are great), and it'd be important to allow their use here, there'll always be packages that don't have valid licenses, or that are dev-only tools, not distributed, for which the license doesn't matter. How can these be targeted?
Separately, licenses often only need to be enforced on distributed code - ie, runtime dependencies. is there a way to filter those in the config?
@ljharb I was intentionally vague in values. Once you allow Does that answer your question, or are you asking about the potential of using installable license lists? I would not be opposed and did consider this but didn't want to go overboard with the initial proposal. I would personally love to see installable license lists, since this would enable something like |
ah, thanks, i didn't read carefully enough. so "module" is for allowed modules - are there disallowed modules? |
@ljharb this is the structure:
There is currently not a way to allow a module explicitly through "allow". What is the use case that isn't already covered? |
I think that "module" needs to support an optional version specifier range - for example, maybe v1 doesn't have a specified license (but is MIT) but v2 is MIT and v3 is GPL, you wouldn't want all versions to be allowed, since then v3 would be allowed, but you also wouldn't want to error on v1? |
@ljharb would this work for you?
Default could be Alternatively (I think this would probably be a bad idea but feel I should propose it):
|
To be clear, what I think is ideal is the config format that licensee already has - is there a reason we can't adopt that, since npm already uses licensee? |
If folks would like me to rewrite this with the licensee format, that's fine. I would prefer to get more input that it's the direction folks would want to go before I do that work 👍🏻 |
Would be anything parsable by Is there anything we can take from the audit resolve spec which can be applied here? As I mentioned on the call, it seems like a "snooze for 1 month" style is really what is effective to reduce noise. If you add to "ignore" it will rely on you remembering to check again once the update from the transitive 3 levels deep gets propagated up to you, this will lead to stale info here I think. |
@isaacs would genuinely appreciate your feedback on this in text, if you have any, before I start editing so I can make sure I don't miss anything - I know you had some thoughts on the RFCs call but my brain did not adequately capture them in a way that I can turn them into a checklist. And no rush, just wanted to make sure I get the request in and share the reason I've not continued to update. |
Action item from the OpenRFC Call:
|
@ruyadorno it would be nice to hear expanded thoughts on that. Do the project's maintainers want to use the JSON format of licensee? Literally just pull in Licensee as a dep and provide a command as an alias for it? Something else? |
I'm really sorry about the vague comment @bnb but it was very intentional 😬 I was very stressed out running the call and I honestly don't remember exactly what were the points, just commented here for the sake of not losing track of it (also remembered you mentioning you'd like textual comments for actionable things) |
Any ideas on how we could progress this? Is just adding a dep on licensee a thing we could do? Or read that format? Happy to work on this, I'm just not sure what next steps requests are from the npm team. |
Adding |
next step discussed in today's meeting:
It was suggested that an independent command or npm ls output would make sense. Addressing those two, respectively:
|
I've updated the RFC to reflect using |
Updated further with the |
remaining unresolved question:
are we okay with introducing an |
worth noting, I've also opened an issue that looks like it'll result in a PR from me this week to licensee to replace |
I'd prefer having "only not package.json" - it avoids inflating the size of the packument/published package, and it avoids adding more stuff into package.json. However, some folks might prefer avoiding "another config file", so I'm not sure what would be best. |
depending on how #564 goes, it might be worth simply waiting for that to be implemented and the subsequent command mapping feature, so as not to waste time on implementing "normally" and then migrating. |
Given that licensee already exists, I’m not sure what the value is of waiting? |
I explored doing it with licensee in a pairing session with @izs and the conclusion we came to was that doing so was massively limiting, given that it would preclude a lot of the benefits you get from doing such a thing with Arborist, a number of which are baseline expectations folks would have of an npm CLI command. |
Ah, that's new information, thanks. |
What / Why
It would be ideal to provide license tooling within the CLI itself rather than leaving it to ecosystem OSS tooling or enterprise-grade registry tools/services to implement it.
Edit: Worth noting that much of the discussion until 5/5/2021 is outdated and reflects an older version of this proposal.